Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

Codecov Report

Merging #8382 into master will increase coverage by 0.02%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #8382      +/-   ##
==========================================
+ Coverage   64.27%   64.29%   +0.02%     
==========================================
  Files         276      276              
  Lines       11700    11707       +7     
  Branches     2859     2864       +5     
==========================================
+ Hits         7520     7527       +7     
  Misses       3549     3549              
  Partials      631      631

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8054b94...89b1538. Read the comment docs.

Hey @JoshRosenstein, thanks for taking care of this!

Discuss if we we want separate matchers or not and if we want to allow mixed types to compare.

I think using the existing matchers is the best way of doing this, let's see if someone wants to argue against it. Not yet sure if comparing numbers to bigints should be allowed, I think I tend towards no because you should know what kind of data type you're dealing with and this might help uncover inconsistencies in user code. It's also not really hard to convert to BigInt.

All numeric checkers can have BigInit support baked in besides toBeCloseTo, therefore is why I had to keep ensureNumbers and add ensureNumbersOrBigInt

I think toBeCloseTo should support it too! :)
It's not limited to decimal places, you can do expect(12).toBeCloseTo(10, -1) as well. This might be a rare use case, but I think it would be consistent to support BigInts there too, plus it allows us to have only ensureNumbers which deals with all kinds of numbers instead of two different variants. Edit: could even make it a matcher error when attempting to use BigInts with a precision > 0.

Comparing bigint to number in toBeGreaterThan toBeGreaterThanOrEqual toBeLessThan toBeLessThanOrEqual is reasonable expectation (pardon pun :) given specification for operators:

https://github.com/tc39/proposal-bigint#comparisons

Numbers and BigInts may be compared as usual. [examples of <, >, >=]

They may be mixed in arrays and sorted.

I think toBeCloseTo would be limited to same type for received and expected, both number or both bigint, because receivedDiff is computed by subtraction.

We would also need to replace Math.abs with unary minus, if the difference is negative.

Unrelated, I noticed this week an existing snapshot fails for toBeCloseTo in Node 12.0.0 :(

- Expected difference: not < 0.000005
+ Expected difference: not < 0.0000049999999999999996

Can confirm in Chrome 74 for Math.pow and ** operator but Firefox 67 still outputs 0.000005

After thinking more about toBeCloseTo and fixing a problem in #8389 and especially because meaningful precision for integers is a double negative, I suggest we leave it alone. If there is need for BigInt version, then a new matcher in jest-community seems like a reasonable solution.

EDIT: Sorry for the run-on sentence. My biggest concern is about meaning of precision:

• for number: majority use case is digits to right of decimal point is positive precision argument
• for number: minority use case for digits to left is tolerable to be negative precision argument
• for bigint: no use case for digits to right of decimal point
• for bigint: only use case for digits to left ought to be positive precision argument, but it does not seem reasonable to invert the meaning based on type of received and expected arguments, does it?

A smaller concern is need for function overloading in TypeScript to constrain both received and expected to have same type and imperative logic in function to enforce it in untyped environment.

Yeah, toBeCloseTo was question mark for me since the optional precision argument would then be a requirement for comparing integers, and then we would have to separate the logic between numbers & bigint since expected and received can be either or, if either is a bigint, would need to cast the number as a bigint before calculating receivedDiff, also as @pedrottimark mentioned, would need to replace any Math. methods that use expected/received, in this instance Math.abs . So that's what I was hesitant about and decided to go the route on creating ensureNumbersOrBigInt. However it would be less code to remove ensureNumbersOrBigInt, and add the seperate logic to handle bigint within toBeCloseTo...

For the case of including bigint within toBeClose, and removing ensureNumbersOrBigInt, ensureExpectedIsNumberOrBigInt, ensureActualIsNumberOrBigInt

the optional precision argument would then be a requirement for comparing integers

the workaround for this would be to set a default precision within the separated logic rather within the arguments and toBeCloseTo would look something like:

toBeCloseTo(
    this: MatcherState,
    received: number | bigint,
    expected: number | bigint,
    // Set to NaN in order to identify later if precision was passed or not
    precision: number = NaN,
  ) {
    const matcherName = 'toBeCloseTo';
    const secondArgument = arguments.length === 3 ? 'precision' : undefined;
    const options: MatcherHintOptions = {
      isNot: this.isNot,
      promise: this.promise,
      secondArgument,
    };
    ensureNumbers(received, expected, matcherName, options);

    let pass = false;
    let expectedDiff: number | bigint = 0;
    let receivedDiff: number | bigint = 0;

    //If both arent numbers, then one might be bigint
    if (typeof received === 'number' && typeof expected === 'number') {
      if (received === Infinity && expected === Infinity) {
        pass = true; // Infinity - Infinity is NaN
      } else if (received === -Infinity && expected === -Infinity) {
        pass = true; // -Infinity - -Infinity is NaN
      } else {
        // Set default precision for numbers
        precision = isNaN(precision) ? 2 : precision;
        expectedDiff = Math.pow(10, -precision) / 2;
        receivedDiff = Math.abs(expected - received);
        pass = receivedDiff < expectedDiff;
      }
    } else {
      // Set default precision for big integers
      precision = isNaN(precision) ? -2 : precision;
      if (precision >= 0) {
        //Should we throw some error for positive precision??
        // Or should we remove -precision from expectedDiff below
        throw new Error(
          matcherErrorMessage(
            matcherHint(matcherName, undefined, undefined, options),
            `${EXPECTED_COLOR(
              'precision',
            )} value must a negative number for BigInts`,
            printWithType('Precision', precision, printExpected),
          ),
        );
      }
      expectedDiff = BigInt(Math.pow(10, -precision) / 2);
      // cast as BigInt ensuring neither are a number to maintain precision
      receivedDiff = BigInt(expected) - BigInt(received);
      receivedDiff =
        receivedDiff >= 0 ? receivedDiff : receivedDiff * BigInt(-1);
      pass = receivedDiff < expectedDiff;
    }
...

Although its a rare case, I'm starting to lean towards this, rather than adding the extra checkers in jest-matcher-utils

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.